Get warp compiling on FreeBSD#9362
Conversation
wayland-client, cctk, fontconfig, x11rb, native-dialog, notify-rust, the WindowingSystem enum and platform/linux all build and run on FreeBSD with the same code we already use for Linux. Replace `target_os = "linux"` with `any(target_os = "linux", target_os = "freebsd")` across app/ and crates/ so FreeBSD hits the Linux branches instead of the cross-platform no-op fallbacks. InputFlags::IUTF8 in local_tty/unix.rs stays Linux/macOS only because nix does not expose that termios flag on FreeBSD. Linux, macOS, and Windows builds are unaffected: every cfg already evaluated true on Linux and continues to.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. I requested changes on this pull request and posted feedback. Comment You can view the conversation on Warp. I approved this pull request. No matching stakeholder was found for the changed files, so no human reviewers were requested. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR broadly widens Linux cfg gates so the existing desktop/Linux implementation is compiled on FreeBSD. The mechanical cfg changes are mostly low-risk, but two changed runtime paths now treat FreeBSD as Linux for downloadable artifacts/update mechanisms.
Concerns
- FreeBSD is routed through the Linux autoupdate implementation, which uses Linux AppImage/package-manager release assets and can offer an incompatible update path.
- FreeBSD selects Linux GNU rust-analyzer assets, which are not native FreeBSD binaries and will fail when Warp installs/executes them.
- Security pass: no direct security vulnerabilities found in the changed lines.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| if #[cfg(target_os = "macos")] { | ||
| mac::download_update_and_cleanup(&version_info, &update_id, last_successful_update_id.as_deref(), server_api.http_client()).await | ||
| } else if #[cfg(target_os = "linux")] { | ||
| } else if #[cfg(any(target_os = "linux", target_os = "freebsd"))] { |
There was a problem hiding this comment.
| "rust-analyzer-x86_64-apple-darwin.gz" | ||
| } | ||
| #[cfg(all(target_os = "linux", target_arch = "x86_64"))] | ||
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "x86_64"))] |
There was a problem hiding this comment.
There was a problem hiding this comment.
Overview
This PR broadly widens Linux-only cfg guards so FreeBSD builds reuse the existing Linux/X11/Wayland/platform implementations.
Concerns
- FreeBSD now selects Linux rust-analyzer release assets, which will install binaries that do not run on FreeBSD.
- The FreeBSD memory-footprint path now uses Linux
/proc/self/statusfields, so it will report 0/empty data on normal FreeBSD systems.
Verdict
Found: 0 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "x86_64"))] | ||
| { | ||
| "rust-analyzer-x86_64-unknown-linux-gnu.gz" | ||
| } | ||
| #[cfg(all(target_os = "linux", target_arch = "aarch64"))] | ||
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "aarch64"))] |
There was a problem hiding this comment.
rust-analyzer-*-unknown-linux-gnu.gz; those Linux ELF binaries will not execute on FreeBSD, so keep FreeBSD unsupported here or select a FreeBSD rust-analyzer asset.
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
💡 [SUGGESTION] This enables the Linux /proc/self/status parser on FreeBSD, where those VmRSS/VmSwap fields are not available; keep FreeBSD on a fallback or use a native FreeBSD memory API so telemetry is not always zero/empty.
rudrabhoj
force-pushed
the
freebsd-fixes
branch
from
035a80c to
df15f82
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
rudrabhoj
force-pushed
the
freebsd-fixes
branch
from
df15f82 to
1e0a6cc
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
rudrabhoj
force-pushed
the
freebsd-fixes
branch
from
1e0a6cc to
6ec7128
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I have signed the Contributor License Agreement and fixed problems cited by @oz-agent kindly review again. |
Three of the previous commit's widened cfg sites pulled FreeBSD into code paths that ship Linux-only artifacts or rely on Linux-only filesystems. Pull FreeBSD back out of those, with a real implementation where a portable one exists. `app/src/autoupdate/mod.rs`: revert the cfg widening. The Linux autoupdate path downloads AppImage and package-manager release assets that aren't published for FreeBSD; FreeBSD now falls through to the existing 'not implemented' arm. FreeBSD users update via pkg or source. `crates/lsp/src/servers/rust.rs`: keep FreeBSD detection working (`should_suggest_for_repo`, `is_installed_on_path`, `is_installed_in_data_dir` are OS-agnostic), but make `install` and `fetch_latest_server_metadata` early-return Err on FreeBSD with a message pointing the user at `rustup component add rust-analyzer` or `pkg install rust-analyzer`. The reason auto-download cannot work: warp installs rust-analyzer through `install_from_github`, which pulls from the `rust-lang/rust-analyzer` GitHub release page. That page publishes mac/linux/windows tarballs only; there is no FreeBSD asset to download. The rustup distribution channel (`static.rust-lang.org/dist/`) does build a FreeBSD rust-analyzer as part of the toolchain, and the FreeBSD ports tree ships one as `devel/rust-analyzer`, so either install lands the binary on PATH where warp finds it. `app/src/system/memory_footprint.rs`: split FreeBSD into its own `mod platform` that calls `getrusage(RUSAGE_SELF)` and reports `ru_maxrss`, instead of pretending `/proc/self/status` exists. linprocfs is optional on FreeBSD and almost never mounted; the previous widened cfg would have made the function silently return 0.
rudrabhoj
force-pushed
the
freebsd-fixes
branch
from
6ec7128 to
3aceb2c
Compare
|
I analyzed the request but did not produce any changes. Powered by Oz |
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR broadens many Linux-specific cfg gates to also compile the existing Linux paths on FreeBSD, plus adds a FreeBSD memory-footprint fallback and disables rust-analyzer auto-install metadata fetches where upstream has no FreeBSD asset.
Concerns
- FreeBSD builds still appear to fail in
app/src/debug_dump.rs: the new FreeBSD cfg referencescrate::autoupdate::linux, butapp/src/autoupdate/mod.rsstill exports that module only on Linux. - No security-specific findings were identified in the changed hunks.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
crate::autoupdate::linux::UpdateMethod reference on FreeBSD, but app/src/autoupdate/mod.rs still exports linux only for target_os = "linux", so FreeBSD builds will fail to resolve the module. Either widen that module cfg too or keep this debug dump block Linux-only.
There was a problem hiding this comment.
Overview
This PR broadens many Linux cfg paths to also compile on FreeBSD and adds small FreeBSD-specific handling for memory reporting and rust-analyzer installation metadata.
Concerns
- The debug dump FreeBSD path now references
crate::autoupdate::linux, but that module is still only declared fortarget_os = "linux", so the FreeBSD build will fail when this file is compiled.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
crate::autoupdate::linux::UpdateMethod, but app/src/autoupdate/mod.rs still declares pub mod linux only under #[cfg(target_os = "linux")], so FreeBSD builds fail; either expose the module for FreeBSD or keep this block Linux-only.
`debug_dump.rs` prints `Package type:` from `crate::autoupdate::linux::UpdateMethod::detect()`. The previous commit reverted `autoupdate` back to Linux-only so the `linux` submodule no longer exists on FreeBSD, but the call site here was widened to `any(linux, freebsd)`. That breaks the FreeBSD build. Narrow the cfg back to Linux-only at the call site. FreeBSD has no package-type detection to report; the line is just a debug-dump nicety, so dropping it on FreeBSD is fine.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR broadens many Linux cfg gates to include FreeBSD and adds a FreeBSD memory-footprint implementation.
Concerns
- The crash-recovery/minidump cfg alias now includes FreeBSD, but the upstream crash-handler/minidumper/minidump-writer stack used by
sentry_minidumpdoes not export FreeBSD implementations, so FreeBSD builds withcrash_reportingenabled will fail.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| fn main() -> Result<()> { | ||
| cfg_aliases! { | ||
| linux_or_windows: { any(target_os = "linux", windows) }, | ||
| linux_or_windows: { any(any(target_os = "linux", target_os = "freebsd"), windows) }, |
There was a problem hiding this comment.
#[cfg(linux_or_windows)] include FreeBSD, so sentry_minidump is compiled there when crash_reporting is enabled; the upstream crash-handler/minidump-writer crates only export Linux/Android, Windows, and macOS implementations, so keep this alias Linux/Windows-only until a FreeBSD minidump backend exists.
There was a problem hiding this comment.
Yes, that's understandable. minidump crate is a project external to warp and it doesn't support FreeBSD yet, it should be fine. I'll soon submit PR to minidump too until then I think this shouldn't be a blocker for a FreeBSD port of warp.
The previous bulk widening pulled FreeBSD into the `linux_or_windows` cfg alias in `app/build.rs` and `crates/warp_cli/build.rs`. That alias gates `mod sentry_minidump` and the `enable_crash_recovery` cfg, both of which would compile the upstream `crash-handler` / `minidumper` / `minidump-writer` stack, none of which has FreeBSD support today. FreeBSD builds with `--features crash_reporting` would fail at compile time. Narrow `linux_or_windows` back to `any(target_os = "linux", windows)`. That neutralises the minidump path on FreeBSD. Cascading consistency: `WorkspaceBanner::WaylandCrashRecovery`, `WorkspaceAction::DismissWaylandCrashRecoveryBannerAndOpenLink`, the `crash_recovery` and `crash_reporting` modules and their match arms were all widened to FreeBSD by the bulk sed but only carry meaning under `enable_crash_recovery`. Revert those cfgs back to `target_os = "linux"` so the variants and arms stay in sync with the alias. A future port of crash reporting to FreeBSD will need to either land FreeBSD support upstream in `crash-handler`/`minidumper`/ `minidump-writer`, or write a FreeBSD-native minidump path. When that arrives, widen the `linux_or_windows` alias and the variants/arms together. Linux, macOS, and Windows are unaffected: the alias and the reverted cfgs all evaluated true on Linux before this change and continue to.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR broadens many existing Linux-specific compile-time gates so the FreeBSD target uses the same Unix/Freedesktop/X11/Wayland code paths where applicable, with a FreeBSD-specific memory footprint implementation and rust-analyzer auto-install fallback behavior.
Concerns
- No blocking correctness issues found in the changed lines.
- Supplemental security pass: no security-impacting issues found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
1 participant
Most
target_os = "linux"cfg sites in this tree gate code that already works fine on FreeBSD. wayland-client, cctk, fontconfig, x11rb, native-dialog, notify-rust, and the WindowingSystem enum all compile and run there using the existing Linux branches. Widening those cfg guards fromtarget_os = "linux"toany(target_os = "linux", target_os = "freebsd")is enough to get a working build.The change is mechanical: 79 files across
app/andcrates/, all of them just adjusting the cfg.One Linux-only carveout:
InputFlags::IUTF8inapp/src/terminal/local_tty/unix.rs. nix does not expose that termios input flag on FreeBSD, and the PTY works without it.Linux, macOS, and Windows are unaffected by construction: every widened cfg already evaluated true on Linux and continues to; neither macOS nor Windows ever matched these guards.
Tested by building
warp-osson FreeBSD 16-CURRENT amd64 with rust 1.92 and launching it under wayland (niri).